Skip to content

fix: xblock-poll's export to CSV feature is not working [TNL-8370] [MNG-2273]#28019

Merged
AhtishamShahid merged 1 commit intoopenedx:masterfrom
open-craft:braden/fix-xblock-poll
Jun 30, 2021
Merged

fix: xblock-poll's export to CSV feature is not working [TNL-8370] [MNG-2273]#28019
AhtishamShahid merged 1 commit intoopenedx:masterfrom
open-craft:braden/fix-xblock-poll

Conversation

@bradenmacdonald
Copy link
Contributor

Summary

xblock-poll's celery task was broken, then fixed in #23700, then broken again by #25479. This fixes it again.

Description

XBlock-poll contains a feature to download a CSV of poll responses. It runs asynchronously in the LMS's celery workers.

Screen Shot 2021-06-23 at 5 43 06 PM

However, we received a report (TNL-8370) that it is currently broken. Attempting to export the results to CSV only shows an error, which may appear in the UI or only in the browser console:

Screen Shot 2021-06-23 at 5 44 45 PM

The reason for this error is that xblock-poll's celery tasks (in tasks.py) are not being registered. They were previously registered because the default LMS settings in common.py explicitly set CELERY_IMPORTS = ('poll.tasks', ...), but https://github.com/edx/edx-platform/pull/25479 removed all explicit celery imports from settings.

I think the reasoning was that all django apps would have their tasks.py auto-detected, but poll is not a django app. I tried making the main poll.py file import tasks.py, but it appears that the LMS celery workers do not load XBlock code at all unless they have some reason to, so I couldn't find any way to register these tasks only by changing the XBlock code.

I'm not sure if there's a better way to make these tasks detected, but this way definitely works.

Supporting information

Testing instructions

It took me a while to figure out how to run celery workers on the docker devstack - am I missing some documentation?

First, use make lms-shell then edit /edx/etc/lms.yml. Make sure CELERY_QUEUES is as follows:

CELERY_QUEUES:
- edx.lms.core.default
- edx.lms.core.high
- edx.lms.core.high_mem

(for some reason, on my devstack, these all had wrong values like edx.lms.core.defaultedx.lms.core., while CELERY_DEFAULT_QUEUE had a correct value, resulting in the worker never seeing the tasks)

Second, add this to lms/envs/private.py:

CELERY_ALWAYS_EAGER = False
BROKER_URL = 'redis://:password@edx.devstack.redis:6379/'

Third, start Redis with make redis-up

Fourth, restart the LMS.

Fifth: from make lms-shell, start a celery worker with

DJANGO_SETTINGS_MODULE=lms.envs.devstack_with_worker celery worker --app=lms.celery:APP

Sixth: add an xblock-poll to a course, answer it so it has some data, and try to export the CSV of results. It should fail. In the logs of the celery worker you should see Received unregistered task of type 'poll.tasks.export_csv_data'..

Seventh: Check out this branch, and restart the celery worker. Try again - it should succeed.

Deadline

None, this has been an issue for months and there is a workaround that can be used in the meantime.

Other information

Nope.

@openedx-webhooks openedx-webhooks added core committer open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Jun 24, 2021
@openedx-webhooks
Copy link

Thanks for the pull request, @bradenmacdonald! I've created OSPR-5879 to keep track of it in JIRA.

As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.

@edx-status-bot
Copy link

Your PR has finished running tests. There were no failures.

Copy link
Contributor

@saadyousafarbi saadyousafarbi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. @AhtishamShahid were you able to test it out with the provided Testing Instructions?

@AhtishamShahid AhtishamShahid merged commit e2a867e into openedx:master Jun 30, 2021
@openedx-webhooks
Copy link

@bradenmacdonald 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

@openedx-webhooks
Copy link

@ormsbee, @kdmccormick: thought you might like to know that bradenmacdonald merged this pull request.

@openedx-webhooks openedx-webhooks added merged and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Jun 30, 2021
@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@natabene
Copy link
Contributor

@bradenmacdonald Thank you for your contribution.

@bradenmacdonald bradenmacdonald deleted the braden/fix-xblock-poll branch July 5, 2021 18:52
blarghmatey pushed a commit to mitodl/edx-platform that referenced this pull request Aug 2, 2021
pomegranited pushed a commit to open-craft/openedx-platform that referenced this pull request Aug 24, 2021
pomegranited added a commit to open-craft/openedx-platform that referenced this pull request Aug 26, 2021
…391)

(cherry picked from commit e2a867e)

Co-authored-by: Braden MacDonald <braden@opencraft.com>
@alfredchavez
Copy link
Contributor

@bradenmacdonald could you port this changes to lilac? as part of BB-4877
CC: @arbrandes for review

@bradenmacdonald
Copy link
Contributor Author

@alfredchavez
Copy link
Contributor

@bradenmacdonald oh, I meant edx:open-release/lilac.master

@bradenmacdonald
Copy link
Contributor Author

@alfredchavez Here you go: https://github.com/edx/edx-platform/pull/29173

edx-community-bot referenced this pull request Nov 1, 2021
…feature is not working [TNL-8370] [MNG-2273] [BB-4877]

## Description

This is a backport of https://github.com/edx/edx-platform/pull/28019 to the Lilac master branch, as [requested](https://github.com/edx/edx-platform/pull/28019#issuecomment-952413371) by @alfredchavez.

Summary:

> xblock-poll's celery task was broken, then fixed in #23700, then broken again by #25479. This fixes it again.


## Supporting information

See original PR.

## Testing instructions

See original PR.

## Deadline

None

## Other information

.
xitij2000 pushed a commit to open-craft/openedx-platform that referenced this pull request Nov 22, 2021
Anas-hameed pushed a commit to edly-io/edx-platform that referenced this pull request Oct 23, 2024
Anas-hameed added a commit to edly-io/edx-platform that referenced this pull request Oct 25, 2024
…590)

Co-authored-by: Braden MacDonald <braden@opencraft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants